Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): implement NRI handler #1674

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(core): implement NRI handler #1674

wants to merge 5 commits into from

Conversation

dqsully
Copy link
Contributor

@dqsully dqsully commented Mar 8, 2024

Purpose of PR?:

Implement NRI support in KubeArmor and mostly fix container confusion issues in BPF-LSM caused by reused Linux namespace IDs.

Does this PR introduce a breaking change? No

If the changes in this PR are manually verified, list down the scenarios covered: Verified functionality in my own sandbox environment.

Additional information for reviewer? :

This NRI solution isn't perfect, but it's much better than the existing containerd and CRI-O solutions at preventing false positives.

For whatever reason, Linux can reuse namespace IDs between containers so long as one container exits before the next one starts. Those namespace IDs are freed as soon as the container's root process exits, and they get created during the runc container initialization process. This means that if KubeArmor's enforcement ends too late for a given container, it could be incorrectly enforcing behaviors on a new container as if they were happening on the old container, including the runc init process to set up a container! On a default "Audit" posture, this is sorta ok, it will just produce false positive alerts, but on a default "Block" posture this will block the new container from being created.

This NRI solution mostly solves these namespace ID overlap issues by starting enforcement on a container after it's already started, and stopping enforcement on a container just before it stops. The only case I can come up with where this doesn't work is if a container crashes unexpectedly, in which case this NRI solution may produce false positive alerts and/or blocks, but it won't be any worse than the existing containerd and CRI-O solutions. That said, I've been running very similar NRI code in my fork of KubeArmor in production for over a month, and I haven't seen any namespace ID overlap issues and I can't cause it to happen synthetically either.

This solution does make one major tradeoff though, which is that there's a very short period of time after a container starts as well as however long it takes for the container to gracefully exit during termination where the container's actions won't be enforced. The existing containerd and CRI-O solutions don't enforce the very start of a container either, and since NRI is event-based it should both react faster on average and use less resources than the current solutions (we did see a significant decrease in CPU after switching from containerd to NRI in my environment).

Checklist:

  • Bug fix.
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Commit has unit tests
  • Commit has integration tests

Also, a quick note about using NRI:

As of writing this, the NRI API is currently in draft status, and while both containerd and CRI-O support NRI, it's not enabled by default in either runtime.

In my case, my company uses EKS with self-managed node groups (so we can tweak configuration as needed) and to enable NRI in the EKS nodes running containerd, we had to both update to the latest AMI and also tweak the user data of the EC2 instances to be the following script:

#!/bin/bash
set -o xtrace

KUBELET_CONFIG=/etc/kubernetes/kubelet/kubelet-config.json

# Enable the NRI plugin for containerd by modifying the containerd EKS source config file before the bootstrap script uses it
cat <<EOF >>/etc/eks/containerd/containerd-config.toml
[plugins."io.containerd.nri.v1.nri"]
    # Enable NRI support in containerd.
    disable = false
    # Allow connections from externally launched NRI plugins. (through the NRI unix socket)
    disable_connections = false
    # plugin_registration_timeout is the timeout for a plugin to register after connection.
    plugin_registration_timeout = "5s"
    # plugin_request_timeout is the timeout for a plugin to handle an event/request.
    plugin_request_timeout = "2s"
    # socket_path is the path of the NRI socket to create for plugins to connect to.
    socket_path = "/var/run/nri/nri.sock"
EOF

# Start the node
/etc/eks/bootstrap.sh

Hopefully NRI is stabilized and enabled by default in containerd and CRI-O soon, but until then I hope this extra information will help anyone else who needs to use NRI.

Signed-off-by: Prateek <prateeknandle@gmail.com>
…cted later than k8s event

Signed-off-by: Prateek <prateeknandle@gmail.com>
Signed-off-by: Prateek <prateeknandle@gmail.com>
Signed-off-by: Prateek <prateeknandle@gmail.com>
Signed-off-by: Prateek <prateeknandle@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: P2 - PR Ready for review
Development

Successfully merging this pull request may close these issues.

2 participants